-
Notifications
You must be signed in to change notification settings - Fork 76
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: change the type to c_char so it can be compiled for aarch64 #337
Conversation
Thanks @cschin for these changes, there seem to be formatting issues, just |
Pull Request Test Coverage Report for Build 1383999223
💛 - Coveralls |
Can you rebase against |
just did. sorry the commits are a bit messy. |
Yeah, thanks, I noticed, would you mind cleaning (squashing) them up a bit? |
Also, could you paste the compile errors you were seeing over here for future reference? I'll probably test your changes with my toy example: https://github.com/brainstorm/s3-rust-htslib-bam |
This is the errors when I compile the master branch:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense to go from i8
to c_char
. Just a byte in a CStr ;)
Thanks for the PR, very much appreciated!
@brainstorm @tedil Thanks. It will make my |
The PR changes the pointer type to i8 to a pointer type to c_char. When I tested to compile this on AWS graviton CPU, the c_char is actually u8 type. The hard-coded pointer type to i8 causes compiling errors. This should work on Intel and aarch64. (I did not test compiling the code on macOS though.)